Check for wp_cache_flush_group() before calling it - #129
Conversation
`wp cache flush-group` asked `wp_cache_supports( 'flush_group' )` whether group flushing was available, then called `wp_cache_flush_group()`. Both arrived in WordPress 6.1, so the version question was covered, but the guard tested a different function than the one being called: `wp_cache_supports()` is answered by the object cache drop-in, and a drop-in that reports support without defining `wp_cache_flush_group()` would have caused a fatal rather than the intended error message. Check for the function itself as well. That is also the guard johnbillion/wp-compat understands, so the single error it reported for this package is resolved without needing an ignore entry. Co-authored-by: Pascal Birchler <[email protected]>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthrough
ChangesCache group flushing
Estimated code review effort: 1 (Trivial) | ~2 minutes Merge Risk: ⚪ Minimal · up to This localized change prevents a fatal error when cache support is advertised without the required function and preserves existing behavior otherwise; no actionable merge-blocking risk remains. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Some tools did not complete. Review the errors below. 🔧 PHPStan (2.2.8)Invalid configuration: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
The
johnbillion/wp-compatPHPStan extension that now ships withwp-cli-testsreported a single error for this package:wp cache flush-groupguarded that call like this:wp_cache_supports()andwp_cache_flush_group()both arrived in WordPress 6.1, so the version question was already covered and this is not a compatibility bug against the 4.9 baseline. But the guard tests a different function than the one being called, and that gap is not purely theoretical:wp_cache_supports()is answered by the object cache drop-in, and a drop-in that reportsflush_groupsupport without definingwp_cache_flush_group()would produce a fatal instead of the intended error message.So rather than adding an ignore entry, the condition now also checks for the function it actually calls. That closes the drop-in gap and, since
function_exists()is a guard wp-compat understands, resolves the error at the same time — this package needs no wp-compat ignores at all.Behaviour
Unchanged everywhere it can be observed. On WordPress 6.1+ the function exists, so the added check is always true and the guard behaves exactly as before. Below 6.1 the command already errored with the same message. The difference only shows up for a drop-in that advertises support it does not implement, where a fatal becomes
Error: Group flushing is not supported.Tests
No changes. The existing
flush-groupscenarios infeatures/cache.featureare tagged@require-wp-6.1and continue to pass unaffected, since the added condition is always satisfied there.Verification
Run locally against the same dependency versions CI resolves (
johnbillion/wp-compat2.0.0,php-stubs/wordpress-stubsv6.9.4,wp-cli/wp-cli-testsv5.2.3):composer phpstan—[OK] No errorscomposer phpcs— cleanphp -l— cleanGitHub Actions was failing to allocate runners across the org while this was written, so CI may need a re-run once that clears.
Generated by Claude Code
Summary by CodeRabbit